-
Notifications
You must be signed in to change notification settings - Fork 923
OpenStack Nova 1.1 driver - Feedback only - not ready for merge #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Make way for multi-versioned OpenStack modules.
That's already done in _populate_hosts_and_request_paths().
With multiple API versions, the class may be different. It's also conceivable that different connection classes may eventually be mixed and matched with different driver classes, especially in the case of OpenStack w/ Keystone.
Version-specific subclasses will inherit from this. I expect more will be moved into it in coming commits.
Note 1: No "Base" in the class name because it is useable as-is. Note 2: The OpenStack base classes I'm writing will be JSON-based. Not wanting to rewrite the (soon-to-be-cut?) XML-based 1.0 driver, that will instead inherit and override for XML where necessary, such as happens here.
I noticed this problem frequently: a Connection's request() instantiates and response, then sets the responses connection attr to itself; however, many of the Responses have __init__s that call their parse_body or parse_error which refers to its own connection object to get details for error output. Fail. Have to pass the connection to the constructor to be available early enough.
Courtesy of PyDev.
HTTP spec states that the Host header must contain the port number if it's non-standard (see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.23). The incorrect override that was present here was breaking the managerment urls in OpenStack auth responses. httplib does the right thing if we let it. Also, the "connection =" removal was because it's shadowed by a following assignment.
Auth 1.1 is not necessary. Auth 1.1 servers accept 1.0 authentication, which gives just as much data (i.e. the same management API URLs). Note: I'll fix the tests later. The Connections now also allow a tenant_id (which appears to be the new name for project_id, so I'll go with it) to be passed in and honored (this is necessary for Alpha customers).
Not all Auth versions need the /auth (e.g. Alpha doesn't). Better to use the full URL as the setting so they can be treated the same.
At least for the OS compute classes, that is. At some point, I hope someone fixes the other OS service drivers and moves this baseward.
Since most OS stuff will be JSON-based, I put a shared encode_data as far baseward as I dared without risking other stuff (someone should look at the other services, though). The 1.0 driver is XML-based, so I had to override it there. As long as I was doing that, I cleaned up all of the places where each and every method had been doing the encoding itself.
All basic ops, most of the extended ones. I'll almost certainly add a few more.
This effectively makes the OpenStack driver appear as a single driver that supports all versions, specified during instantiation (except it's much easier to maintain).
Plus a little bit o' PEP8.
Member
|
(just doing some pull request cleanup) @Manganeez - Those changes have been merged a long time ago, can you please close this pull request? Thanks! |
Kami
added a commit
to Kami/libcloud
that referenced
this pull request
Dec 7, 2022
…a..5e29e4749 5e29e4749 README: prep for v1.0.1 5ed8d3149 action: remove deprecated use of set-output (apache#29) 2e332741f Add FAQ entry for pipenv support (apache#28) fc80c9a7d requirements: pip-audit 2.4.4 (apache#27) ac6a629be README: update slugs from trailofbits to pypa (apache#26) 2d48142e4 action.py: Remove unused line (apache#25) 6257a8afd Add Semgrep CI 4a89a3810 action, templates: switch to a template (apache#24) git-subtree-dir: .github/actions/gh-action-pip-audit git-subtree-split: 5e29e474921e0cd12d5b02b4bf4a6ebebe2497aa
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Still have tests to add and clean up, but wanted to open a pull request in case anyone has feedback on any of the refactoring or the factory use, which, as I said in the commit, "effectively makes the OpenStack driver appear as a single driver that supports all versions, specified during instantiation (except it's much easier to maintain)."
This is for LIBCLOUD-83.